Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: no IPFS_JS_EXEC during 1st pass of test-external #498

Closed
wants to merge 1 commit into from

Conversation

lidel
Copy link
Member

@lidel lidel commented Jan 14, 2020

Parent issue: ipfs/js-ipfs#2706

This fix ensures IPFS_JS_EXEC is ignored during the first run of npm test in test-external.
It will work as expected during the second time npm test is run.

Motivation

We want to run ipfs-webui tests in ipfs/js-ipfs#2706,
however, for some reason E2E tests do not use the ipfs from file:*.tgz defined in package.json.

I am not sure what is happening exactly, connect-deps does not do the trick, only setting IPFS_JS_EXEC works. Either way, this seems to be a useful fix to merge anyway, as it removes potential side effect from initial pass.

cc @hugomrdias

This fix ensures custom js-ipfs is not used for the first run
of `npm test`, even if IPFS_JS_EXEC is defined.

It will be present only during the second `npm test`

License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
@achingbrain
Copy link
Member

LGTM but presumably we want to leave the rest of the env intact?

E.g instead do:

env: {
  ...process.env,
  // ensure custom js-ipfs is not used for the first run
  IPFS_JS_EXEC: undefined
}

@lidel
Copy link
Member Author

lidel commented Jan 16, 2020

@achingbrain the rest of env is left intact, execa (lib used for exec) extends by default:

https://www.npmjs.com/package/execa#env:

env

Type: object
Default: process.env

Environment key-value pairs. Extends automatically from process.env.
Set extendEnv to false if you don't want this.

@lidel
Copy link
Member Author

lidel commented Jan 16, 2020

@hugomrdias do you mind merging and releasing, so we can unblock ipfs/js-ipfs#2706?
(I believe fix for the connect-deps issue could land separately, but if you want me to wait for it, its fine too)

@hugomrdias
Copy link
Member

the main reason for this change was the webui tests and that was fixed with ipfsd-ctl v1.0.6.
i prefer to not mess with env without a clear reason, so im closing this without merging. In the future if we find another reason to do this ill merge this change.

thank you all for helping.

@hugomrdias hugomrdias closed this Jan 17, 2020
@hugomrdias hugomrdias deleted the feat/test-external-with-env branch April 20, 2020 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants